fix(driver-sql): render the hash-shadow NULL-safe duplicate report through formatDuplicateGroups - #16579

Fixes #16289

What

packages/drivers/driver-sql/src/sql-driver.ts — the hash-shadow arm's NULL-safe branch was the fourth site that renders duplicate groups and the only one still hand-rolling the shape formatDuplicateGroups (#14902) exists to own. It now calls the helper. One hunk, seven lines out, one line in.

The surrounding framing is byte-identical to the other two durability logs, mechanically:

$ grep -o -F 'report = ` Conflicting group(s): ${formatDuplicateGroups(duplicates)}.`;' \
    packages/drivers/driver-sql/src/sql-driver.ts | sort | uniq -c
      3 report = ` Conflicting group(s): ${formatDuplicateGroups(duplicates)}.`;

Three occurrences of one distinct string (:11941 this arm, :11994 and :12059 the plain-unique logs), plus the drift entry at :10844 which embeds the same helper output in a drift message. No wording was improved at any site, here or elsewhere.

The drift, re-derived on this branch's own head

All five anchors matched the line numbers recorded on the card, because this branch is cut from the same commit the triage measured (ce8caba9). They were re-derived by symbol, not trusted.

So two durability logs about the same failure class, emitted from the same catch, disagreed on how they say "there are more" — the exact drift the helper's own docblock names as the reason it exists.

Adopting the helper moves the tail and nothing else — the route note asked for this to be checked rather than assumed. The five-group half of the helper is character-for-character the expression that was inlined here (.slice(0, 5), then `(${g.key}) × ${g.rows} rows`, then .join('; ')), so ordering, truncation and per-group spelling are untouched; the probe that feeds both is the same probeNullSafeUniqueDuplicates call with the same arguments. The ablation below shows it from the other side: both legs render five (key) × 2 rows groups over the same fixture and differ only in what follows the last ;.

⛔ The existing pin's green is NOT coverage for this change

sql-driver-12998-shadow-null-safe-key.test.ts asserted the message at :203 only, and only as a prefix:

expect(diagnosis).toMatch(/Conflicting group\(s\):/);

Measured on the pre-change file, with a firing control so the zero is a real zero:

⇒ nothing in the tree could see the overflow tail, so adopting the helper would have silently repaired an unmeasured divergence and a green suite afterwards would have proved nothing about it. Extending coverage was therefore part of the work.

(One correction to the dispatch's reading, which said 24 expect( calls: measured 23, by both grep -c and grep -o | wc -l. The material half — only :203 touches the message, nothing touches the tail — is unchanged.)

The coverage, and why it lives in that file

New live cell in the same pin file, counts the sixth conflicting group in the shared "more group(s)" tail:

• Six conflicting groups is the smallest fixture in which the tail renders at all (formatDuplicateGroups shows five, then counts the rest), so the fixture is six distinct values, each doubled under a NULL organization.
• It asserts both halves of the rendering: exactly five groups shown (× 2 rows occurs 5 times) and the counted tail ; …and 1 more group(s).. The count assertion is what makes it a pin on the shared renderer rather than on one arm's private spelling.
• It carries an explicit 60_000 budget, like the four it() blocks already in that file (driver-sql: 37 more live-DDL it() blocks across 9 files inherit vitest's 5000ms default (same shape as #13688) #13902) — a new cell with no third argument would inherit vitest's 5000 ms default, which is exactly the exposure [finding] driver-sql's live PG + MySQL matrix runs under vitest's default 5000ms — the only live-DB driver in the repo with no testTimeout, and it just dequeued an unrelated PR #16434 is about.

It belongs in this file and not in a cheaper unit test because the arm is only reachable on live MySQL: the hash-shadow route exists because MySQL refuses to key a TEXT column directly, and SQLite and Postgres never refuse. There is no non-live path to this catch.

Firing control: the test FAILS on the unmodified fourth site

Not "the suite is green afterwards". The fix was committed first, then the fourth site alone was reverted to its pre-change bytes, both legs run against the same live MySQL 8.0.46, and the tree restored from HEAD (never a bare git checkout --), with a trap on absolute paths.

On-disk proof, not the editor's exit code — the mutation reproduced main's blob exactly:

HEAD_BLOB=91337b2b75ebf3481f6553f1dda318db2b820575     # the fix
PRE   helper-call-lines=3  drift-tail=0
POST  helper-call-lines=2  drift-tail=1                 # anchored counts, both directions
MUT_BLOB=393a83f1a55b1e3c495fe50464314bd51f481f55       # == the pre-image blob in this PR's own diff header
LEG_A_EXIT=1                                            # RED on the pre-fix site
RESTORE_DIFF_EMPTY_EXIT=0                               # git diff HEAD -- THE_FILE is empty
REST_BLOB=91337b2b75ebf3481f6553f1dda318db2b820575      # == HEAD_BLOB
RESTORED helper-call-lines=3  drift-tail=0
LEG_B_EXIT=0                                            # GREEN with the fix

Leg A, the whole point — the drift printed by a real MySQL, and the four pre-existing tests passing straight through it:

 ✓ collides two NULL-organization rows under an org-scoped shadow unique
 ✓ leaves plain composite key parts un-coalesced
 ✓ diagnoses existing NULL-org duplicates instead of failing the boot unexplained
 ✓ names a NULL-organization duplicate in declared terms, never as a collision
 × counts the sixth conflicting group in the shared "more group(s)" tail
   → expected '[sql-driver] cannot create hash-shado…' to match /; …and 1 more group\(s\)\./

and the message it was matched against ended:

… (organization_id="__global__", v="eee…") × 2 rows; …and 1 more. The constraint 'COALESCE(organization_id, '__global__'), v' is NOT enforced until …

…and 1 more. — the divergence, live, in the shipped text. Leg B renders …and 1 more group(s). and the file is Tests 5 passed (5).

⭐ The four pre-existing tests passing on both legs is the measurement the card asked for, demonstrated rather than argued: this change is invisible to every test that existed, so the pin's green is not coverage for it.

Which tree the legs ran on, stated rather than implied: the ablation ran at fdf3defef, before the origin/main merge and before a later commit bound the fixture to a variable so tsc accepts it. The subject under test did not move across that range — git rev-parse fdf3defef:…/sql-driver.ts and a2cd9f58e:…/sql-driver.ts are the same blob 91337b2b7, and git diff --quiet between the two commits on that path exits 0. The test file's only delta is that binding (5 insertions, 1 deletion); every assertion line is unchanged.

A second, confirming run of the same two legs on the final head was attempted and not measured: os-verify-lock returned VERDICT queue-timeout (exit 99) · never acquired · waited 540s, the shared heavy-verify lock being held for 824s by a sibling's pnpm --filter @objectstack/spec test && pnpm --filter @objectstack/spec typecheck. Exit 99 is "never got a turn", not a result, and the tree was left unmutated — the legs above stand on the blob identity just stated, and are not re-reported as if they had run twice.

Verification

Live MySQL 8.0.46 was provisioned in the dispatch container from the distro archive — the same route sql-driver-15479-…'s header records — and pointed at CI's own URL (mysql://root:root@127.0.0.1:3306/conformance), so the live cells here ran against a real server rather than skipping.

Every heavy run went through scripts/pm/os-verify-lock.sh and every verdict above is that script's VERDICT command-exit line or the runner's own exit written to disk before any pipe — never a bare $? after a | tail.

Repo-wide lint stays CI's; what is here is a proven narrowing. pnpm exec eslint --no-inline-config --format json over the two files this diff touches: exit 0, 2 files linted (count read from the JSON, not asserted), 0 errors, 0 warnings. The narrowing is a measurement rather than a skip because of the third piece: this repo runs one eslint.config.mjs which, quoting its own comment at :326, "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file, test or not" — measured there with a positive control. With no cross-file type information in play, this diff cannot move the verdict on a file it does not touch, so the linted population (the config's own **/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs} minus NEVER_LINTED) is unaffected outside those two files.

Gates

Derived from the tree, not from memory: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands on the final head a2cd9f58e, giving 57 runnable families over the change set it derives itself (3 paths, three-dot against merge base 90e7e6de1).

The branch carries a merge of origin/main 90e7e6de1, which cleared the tool's STALE TREE warning at the time. main moved again while the sweep ran (d1c86a745, a docs/ADR commit), so the final derivation prints the warning once more, naming one stale file: scripts/publish-smoke.sh. Stated rather than hidden — no path of this diff is in it, and CI derives on the merge result anyway.

Each printed command was run byte for byte, its exit code captured to disk before any pipe, and the record reconciled against the tool's own output:

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran ran.list
✓ dispatch-gates --ran: 57 derived famil(ies) accounted for — 57 run, 0 NOT-MEASURED.

55 exit 0. Two exit 3, and 3 is this repo's PREREQUISITE NOT MET — read as NOT MEASURED, never as red and never as pass, in their own words: "⛔ This is NOT a pass and NOT a finding: nothing was measured."

• pnpm check:dual-build-cjs-loads — "this gate reads built output, and some package has no dist/" (74 packages).
• pnpm check:type-check-debt — "25 workspace dependenc(ies) of the ledgered packages have no built type entry point on disk".

Both want the whole workspace built (turbo run build --filter='./packages/*'), which is what lint.yml does before its own step; this container built only driver-sql's dependency closure, per the local-validation scope. Left to CI, recorded as not measured rather than folded into a green.

Outside that 57 the tool lists 39 artifact-roster families, whose silent verdict is a fact about a list rather than about these paths — explicitly not part of the runnable total. Two things were done with that block rather than reading it as clearance:

• 4 of them keep their roster in a directory one of these paths is in, where the tool says the silence is evidence in neither direction. Those four were run: check-changeset-fixed (0), check:authz-resolver (0), check:error-code-casing (0), check:filter-alias-parity (0).
• 9 of them run only the checker's own --self-test, each marked ⚠ checker-health only — NOT a PR verdict. Their green is not read as one here. check:partof-closing-keyword is one of those nine, so the Fixes #16289 line above is verified by CI's PR-context step and not by anything run locally.

Outside the local total by the tool's own declaration, and left to CI: 10 families whose declared population is too wide to place, 5 whose argv takes a value only a CI run supplies, and the always-runs tail.

Scope

Only the hash-shadow NULL-safe arm. The other three call sites were read and left untouched; no other duplicate-group or diagnostics code was widened, and no wording was improved anywhere. .changeset/hash-shadow-null-safe-duplicate-group-tail.md is a patch on @objectstack/driver-sql: the diff changes the text of a message that ships from a released package, so skip-changeset would be wrong (Post-Task Checklist §3).

Clause-② — re-derived from the actual diff: no. A durability log message is not a published contract surface. No exported symbol moves — formatDuplicateGroups stays module-local and unexported, and its docblock says why (SqlDriver's .d.ts carries protected members, so a method there would move a published entry point for a string helper). No accept set widens, no schema or spec key is touched. The diff is one call site plus its coverage.

验收备注

• Filed as driver-sql: initObjects / registerObjectMetadata still omit indexes from a parameter type they read it through — the shape #4311 fixed for tenancy, one key over #16570 (out of scope here, found by this work): initObjects / registerObjectMetadata declare Array<{ name; fields?; tenancy? }> and do not admit indexes, while :9510 reads (obj as any).indexes and drives the whole index-sync path from it. An inline object literal carrying indexes is rejected by tsc; the same object bound to a variable compiles and syncs. It is the shape the comment three lines above that signature already describes for tenancy ([P2] framework: 66 个包用 tsup 构建、无人做类型检查 —— 实测 18 个包共 380 处 code-tier 错误(#4118 的 framework 侧对应) #4311), one key over. The new fixture here binds a variable, like every other fixture in the file — the workaround was load-bearing and undocumented.
• The dispatch's reading of the pin file said 24 expect( calls; measured 23, by both grep -c and grep -o | wc -l. The conclusion it carried is unaffected.
• Triage cross-referenced [finding] driver-sql's live PG + MySQL matrix runs under vitest's default 5000ms — the only live-DB driver in the repo with no testTimeout, and it just dequeued an unrelated PR #16434 (driver-sql sets no testTimeout, so live cells fall back to vitest's 5000 ms default) as the thing to diagnose a live-cell timeout against. Re-derived here: packages/drivers/driver-sql/vitest.config.ts indeed sets no testTimeout — and the four it() blocks in this pin file each carry an explicit 60_000 (driver-sql: 37 more live-DDL it() blocks across 9 files inherit vitest's 5000ms default (same shape as #13688) #13902), so this file is not exposed to that default. The new cell carries the same explicit budget deliberately, so it does not become the file's first 5000 ms cell. No live cell timed out here (the slowest was 248 ms).
• The live server is a side effect worth declaring: MySQL 8.0.46 was installed into the shared dispatch container and left running on 127.0.0.1:3306 with CI's own credentials and database name. Live MySQL cells across that container now run instead of skipping for every agent on it — deliberately not torn down, because a sibling seat is mid-round on this package's live cells and removing the server under it would be the more destructive choice.

Generated by Claude Code